Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tests] Improve testing of FieldSortBuilder #26437

Merged
merged 1 commit into from
Aug 31, 2017

Conversation

cbuescher
Copy link
Member

Currently we don't do much unit testing about the SortField that is created then calling the
SortBuilders build method. Most of this is covered by integration tests somewhere but it
would be good to have some basic checks in FieldSortBuilderTest as well.

This adds testing for the sort order, mode, missing values and checks that nested gets set
in the XFieldComparatorSource when nestedPath and nestedFilter are set on the builder.
Also it is simplifying the setup of the mocked QueryShardContext a bit.

Relates to #17286

@cbuescher cbuescher added :Search/Search Search-related issues that do not fall into other categories >test Issues or PRs that are addressing/adding tests labels Aug 30, 2017
@cbuescher cbuescher requested a review from jimczi August 30, 2017 10:00
@cbuescher
Copy link
Member Author

@jimczi would you mind taking a look since we talked about this already briefly? I got rid of the whole Lucene index setup after our discussion as suggested, this is much clearer. I'll continue with some small additions to the other SortBuilder tests in follow up PRs.

@mattweber
Copy link
Contributor

@cbuescher Please see #26395 as nestedPath and nestedFilter are being deprecated for a new format.

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
We'll need to test the new format for nested:
#26395
but this can be done in a follow up.

@cbuescher
Copy link
Member Author

@jimczi thanks, I will rebase before I merge and look at the incoming changes. I will look at the new nested format in a follow up like suggested. There's also things to add to the geo/script tests if possible.

Currently we don't do much unit testing about the SortField that is created then
calling the
SortBuilders `build` method. Most of this is covered by integration tests
somewhere but it
would be good to have some basic checks in FieldSortBuilderTest as well.

This adds testing for the sort order, mode, missing values and checks that
`nested` gets set
in the XFieldComparatorSource when `nestedPath` and `nestedFilter` are set on
the builder.

Relates to elastic#17286
@cbuescher cbuescher merged commit adad605 into elastic:master Aug 31, 2017
cbuescher added a commit that referenced this pull request Aug 31, 2017
Currently we don't have much unit testing about the SortField that is created then
calling the SortBuilders `build` method. Most of this is covered by integration tests
somewhere but it would be good to have some basic checks in FieldSortBuilderTest
as well.

This adds testing for the sort order, mode, missing values and checks that `nested` 
gets set in the XFieldComparatorSource when `nestedPath` and `nestedFilter` are 
set on the builder.

Relates to #17286
cbuescher added a commit that referenced this pull request Aug 31, 2017
Currently we don't have much unit testing about the SortField that is created then
calling the SortBuilders `build` method. Most of this is covered by integration tests
somewhere but it would be good to have some basic checks in FieldSortBuilderTest
as well.

This adds testing for the sort order, mode, missing values and checks that `nested`
gets set in the XFieldComparatorSource when `nestedPath` and `nestedFilter` are
set on the builder.

Relates to #17286
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Aug 31, 2017
* master:
  Allow abort of bulk items before processing (elastic#26434)
  [Tests] Improve testing of FieldSortBuilder (elastic#26437)
  Upgrade to lucene-7.0.0-snapshot-d94a5f0. (elastic#26441)
  Implement adaptive replica selection (elastic#26128)
  Build: Quiet bwc build output (elastic#26430)
  Migrate Search requests to use Writeable reading strategies (elastic#26428)
  Changed version from 7.0.0-alpha1 to 6.1.0 in the nested sorting serialization check.
  Remove dead path conf BWC code in build
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Search/Search Search-related issues that do not fall into other categories >test Issues or PRs that are addressing/adding tests v6.0.0-rc1 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants